add --verbose flag
#415
add --verbose flag
#415
Conversation
|
The failing test (failing to parse error message on windows) seems to be unrelated to this PR. It is the same issue which is blocking the dependabot. |
|
I think I also fixed the path issue on windows! |
|
Do you mind applying that test path fix to the pending Dependabot PR? It should pass after that and I'll accept the PR. |
|
Appreciate the tests, BTW! |
|
Looks good - tho I can't merge it because it's a Draft PR? |
Thanks for your quick feedback and helps. I'm trying an idea (and simultaneously learning a bit of JS). I'll finalize this in a couple of days. |
|
I'm not sure there's a lot of room to improve upon what you have here, but I will keep an open mind until I see what you're up to. :) |
file names given to and received the library are logged independently linters’s success messages are logged to stdout and fail to stderr
|
@DavidAnson Thanks for keeping an open mind. 😄I hope you are not allergic to bad JS codes! Now the
As you already know, I’m not familiar with idiomatic JS. I’ll appreciate if you can guide me to improve this PR. |
There was a problem hiding this comment.
OK, I see what you're going for and it makes sense in the context of what I've seen from other tools! However, the new behavior raises some more questions and I've left new comments. I will need to look at this again on a bigger screen than my phone, but I think this captures all the significant feedback. Thank you!
| .option('--enable [rules...]', 'Enable certain rules, e.g. --enable MD013 MD041 --') | ||
| .option('--disable [rules...]', 'Disable certain rules, e.g. --disable MD013 MD041 --'); | ||
|
|
||
| program.parse(process.argv); | ||
|
|
||
| if (options.quiet && options.verbose) { | ||
| options.verbose = false; |
There was a problem hiding this comment.
This makes me feel like it's one setting with three values?
| @@ -256,6 +291,9 @@ const files = prepareFileList(program.args, ['md', 'markdown']).filter(value => | |||
| const ignores = prepareFileList(options.ignore, ['md', 'markdown'], files); | |||
| const customRules = loadCustomRules(options.rules); | |||
| const diff = files.filter(file => !ignores.some(ignore => ignore.absolute === file.absolute)).map(paths => paths.original); | |||
| if (options.verbose && !options.stdin) { | |||
| console.log('files to check:', diff.join(' ')); | |||
There was a problem hiding this comment.
This will be all on the same line? That is harder to scan visually.
| @@ -299,6 +337,10 @@ function lintAndPrint(stdin, files) { | |||
| const fixResult = markdownlint.sync(fixOptions); | |||
| const fixes = fixResult[file].filter(error => error.fixInfo); | |||
| if (fixes.length > 0) { | |||
| if (options.verbose && !options.output) { | |||
| console.log('fixing', file); | |||
There was a problem hiding this comment.
If there is a colon above, let's use one here also.
| }) | ||
| ); | ||
| .filter(Boolean); | ||
|
|
||
| let lintResultString = ''; |
There was a problem hiding this comment.
Instead of keeping this, I think it can be removed and recreated if it is needed. It looks like it is empty exactly when the strings array has no elements?
| // @see {@link https://nodejs.org/dist/latest-v8.x/docs/api/process.html#process_process_exit_code} | ||
| // @see {@link https://github.com/igorshubovych/markdownlint-cli/pull/29#issuecomment-343535291} | ||
| process.exitCode = exitCodes.lintFindings; | ||
| return `${result.file}: ✔`; |
There was a problem hiding this comment.
I would expect failing files to end with a little 'x'?
| t.is(result.exitCode, 0); | ||
| }); | ||
|
|
||
| test('--verbose flag with --fix for incorrect file', async t => { |
There was a problem hiding this comment.
Missing test for verbose with broken file (no fix).
| t.is(result.exitCode, 0); | ||
| }); | ||
|
|
||
| test('--output and --verbose with valid input logs nothing to console', async t => { |
There was a problem hiding this comment.
I could argue that verbose should always mean "verbose to console". If so, these two test cases would not be different. Just like I don't like how quiet and verbose overlap, disabling verbose for output seems not obvious?
--verboseflag can be used to ensure linter is actually parsing the files when using globs or for general debugging.PS:
I couldn’t think of a simple solution to logs the relevant messages after each file’s name.